Skip to content

fix fixture path losing suffix after dynamic ID substitution#1549

Merged
tomer-stripe merged 3 commits intostripe:masterfrom
knQzx:fix-fixture-path-verify
May 8, 2026
Merged

fix fixture path losing suffix after dynamic ID substitution#1549
tomer-stripe merged 3 commits intostripe:masterfrom
knQzx:fix-fixture-path-verify

Conversation

@knQzx
Copy link
Copy Markdown
Contributor

@knQzx knQzx commented Apr 16, 2026

Fixes #1340

When using dynamic IDs in fixture paths like /v1/customers/${customer:id}/sources/${bank_account:id}/verify, the /verify suffix was stripped during ID resolution

The bug was in ParsePath in pkg/parsers/parsers.go - the condition for appending the trailing path segment after the last regex match used len(pathParts)%2 == 0, which only works when there is exactly one dynamic ID in the path. With two or more dynamic IDs the modulo check fails and the trailing segment is dropped

Changed the condition to len(pathParts) > len(matches), which correctly appends the trailing segment regardless of how many dynamic IDs appear in the path

Added a test case covering two dynamic IDs with a trailing path suffix

@knQzx knQzx requested a review from a team as a code owner April 16, 2026 17:15
@tomer-stripe
Copy link
Copy Markdown
Collaborator

Trying to figure out how to get github to rerun the required tests so I can squash and merge this

tomer-stripe added a commit that referenced this pull request May 8, 2026
## Summary
- Re-adds `pull_request` trigger to the test workflow
- Fixes fork PRs being unable to satisfy required `test (1.26.0, ...)`
branch protection checks

The test workflow was changed to push-only in #1560, which inadvertently
broke CI for all fork/external PRs. Fork pushes only fire workflows on
the fork repo, so the required checks never appear on community PRs.

This workflow doesn't use secrets, so it's safe to run on fork PRs.

## Test plan
- [x] Verify this PR itself triggers the `test (1.26.0, ...)` checks
- [ ] Verify existing fork PRs (e.g. #1549) get CI runs after merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@tomer-stripe tomer-stripe merged commit 776d7b5 into stripe:master May 8, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bank account verify using a fixture removes /verify from the path when using dynamic IDs

2 participants